Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ISSUE #73] Add remoting service #74

Merged
merged 5 commits into from
Mar 29, 2024
Merged

Conversation

Lambert-Rao
Copy link
Contributor

No description provided.

Comment on lines +20 to +23
public class CreateAclResponse {

Boolean success;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference between Response and Result?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

result have a future of response

Comment on lines +22 to +25
public class GetClientResult {

CompletableFuture<GetClientResponse> getClientResponseFuture;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to warp CompletableFuture<GetClientResponse> into GetClientResult?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for a clearer division of responsibilities

Comment on lines 35 to 36

private List<String> brokerAddress;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be namesrvAddress?

Comment on lines 34 to 37

List<ConfigMetadata> configMetadataList;
private Boolean success;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that configMetadataList is missing a private decorator.

Comment on lines 25 to 28

private GroupMetadata groupMetadata;
private String bootStrapServers;

private String groupName;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be bootstrap instead of bootStrap as bootstrap is a single word.

Is there any relationship between GetOffsetRequest and these two members?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure, what fields are needed for a request, developers who implement these remoting services can modify the fields

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, then lets leave a TODO comment and delete these members.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may finish this part in the next PR.

Comment on lines 30 to 31
//center url
private String registryAddress;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

center naming shall be replaced.

scwlkq and others added 3 commits March 29, 2024 16:08
…q) (apache#66)

* modify RocketmqTopicCore, using SDKManager

* fix style

* delete some variable in RocketmqProperties and TopicProperties & remote additional endPoint name

* remove junit dependency back to console module

* remove unnecessary brackets & revert em-dashboard-pom.xml & move RocketmqProperties to core.dto

* add todo

* move todo to common sdk dependencies & fix style

* add test and junit dependencies in core pom.xml

* revert the indentation in pom.xml & move test related dependencies to bottom in core-pom.xml

* revert indentation in pom.xml

* revert indentation in pom.xml

* remove unnecessary params

* remove params

* remove empty line
@Pil0tXia
Copy link
Member

You may finish the bootStrapServers naming in ResetOffsetRequest in the next PR.

@Pil0tXia Pil0tXia merged commit 13eae3f into apache:dev Mar 29, 2024
2 checks passed
SLSJL pushed a commit to SLSJL/eventmesh-dashboard that referenced this pull request Mar 31, 2024
…to dev

* 'dev' of https://github.com/SLSJL/eventmesh-dashboard:
  [ISSUE apache#73] Add remoting service (apache#74)
  [ISSUE apache#45] Implement methods from storage-plugin.admin(rocketmq) (apache#66)
  [ISSUE apache#71] Reduce log file size and Streamline debug output (apache#72)
  [ISSUE apache#69] Integrate database credentials in auto-deploy (apache#70)
  [ISSUE apache#67] Fix HealthCheckResultMapper which leads to application error
  [ISSUE apache#64] Support automated deployment and Fix runtime packaging errors (apache#65)
  [ISSUE apache#60] add SDK manager (apache#62)
  [ISSUE apache#57] Modify the field, synchronize the modification, and add the mapper method (apache#58)
  [ISSUE apache#29] Set up EventMesh Dashboard Front-end (apache#56)
  [ISSUE apache#49] RocketMQ and Nacos health check (apache#53)
  [ISSUE apache#51] Config Mgmt basic function and config,runtime,store,cluster SQL (apache#52)

# Conflicts:
#	eventmesh-dashboard-view/public/index.html
#	eventmesh-dashboard-view/src/App.tsx
#	eventmesh-dashboard-view/src/index.tsx
#	eventmesh-dashboard-view/src/routes/RootLayout.tsx
#	eventmesh-dashboard-view/src/routes/navigation/Navigation.tsx
#	eventmesh-dashboard-view/src/routes/navigation/NavigationItem.tsx
#	eventmesh-dashboard-view/src/routes/topic/Topic.tsx
#	eventmesh-dashboard-view/src/routes/topic/stats/AbnormalTopicCount.tsx
#	eventmesh-dashboard-view/src/routes/topic/stats/Stats.tsx
#	eventmesh-dashboard-view/src/routes/topic/stats/StatsChart.tsx
#	eventmesh-dashboard-view/src/routes/topic/stats/TopicCount.tsx
#	eventmesh-dashboard-view/src/routes/topic/topic-list/TopicList.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants